Add model-aware reasoning and ChatGPT Pro handoff - #214
Open
Nul-led wants to merge 2 commits into
Open
Conversation
Contributor
📝 WalkthroughWalkthroughThe PR adds a ChatGPT Pro handoff flow that packages thread and repository context into Markdown, introduces per-model reasoning efforts including Max and Ultra, and adds banked rate-limit reset retrieval, consumption, and UI handling. ChangesChatGPT Pro handoff
Model-aware reasoning efforts
Banked rate-limit resets
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant SidebarThreadTree
participant App
participant CodexGateway
participant CodexAppServerBridge
participant ChatGPT
User->>SidebarThreadTree: Select “Continue in ChatGPT Pro…”
SidebarThreadTree->>App: Emit selected thread id
App->>CodexGateway: Request handoff
CodexGateway->>CodexAppServerBridge: POST thread/pro-handoff
CodexAppServerBridge-->>CodexGateway: Markdown and ChatGPT URL
CodexGateway-->>App: Handoff payload
App->>App: Copy Markdown to clipboard
App->>ChatGPT: Navigate opened window
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
PR Summary by QodoModel-aware reasoning picker + ChatGPT Pro handoff from selected thread
AI Description
Diagram
High-Level Assessment
Files changed (13)
|
Code Review by Qodo
1. Dark overrides kept in App.vue
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
Comment on lines
+6181
to
+6187
| :root.dark .pro-handoff-notice { | ||
| @apply border-emerald-800 bg-emerald-950 text-emerald-100; | ||
| } | ||
|
|
||
| :root.dark .pro-handoff-notice.is-error { | ||
| @apply border-rose-800 bg-rose-950 text-rose-100; | ||
| } |
There was a problem hiding this comment.
1. Dark overrides kept in app.vue 📘 Rule violation ⚙ Maintainability
The new pro-handoff-notice dark-theme overrides are implemented as component-scoped :root.dark CSS inside src/App.vue rather than placing the decisive overrides in src/style.css. This violates the requirement to centralize dark-theme fixes for shared/large UI surfaces, increasing the risk of fragmented or fragile theming behavior.
Agent Prompt
## Issue description
The dark-theme overrides for the new `pro-handoff-notice` are implemented via component-scoped `:root.dark ...` rules inside `src/App.vue`, but compliance requires decisive dark-theme overrides for shared/large UIs to be placed in `src/style.css`.
## Issue Context
`pro-handoff-notice` is a global/shared UI notice (fixed overlay) and should follow centralized theming rules to avoid scattered dark-mode overrides.
## Fix Focus Areas
- src/App.vue[6181-6187]
- src/style.css[1-999999]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+565
to
+571
| export async function createChatGptProHandoff(threadId: string, cwd: string): Promise<ChatGptProHandoff> { | ||
| const response = await fetch('/codex-api/thread/pro-handoff', { | ||
| method: 'POST', | ||
| headers: { 'Content-Type': 'application/json' }, | ||
| body: JSON.stringify({ threadId, cwd }), | ||
| }) | ||
| const payload = await response.json() as { data?: Partial<ChatGptProHandoff>; error?: unknown } |
There was a problem hiding this comment.
2. Handoff fetch lacks timeout 🐞 Bug ☼ Reliability
createChatGptProHandoff() performs a POST to /codex-api/thread/pro-handoff without any AbortSignal/timeout, so a stalled bridge/server can leave the “Packaging…” notice visible and the pre-opened blank tab orphaned indefinitely. This is inconsistent with other gateway fetches that already enforce timeouts.
Agent Prompt
### Issue description
`createChatGptProHandoff()` uses `fetch()` without a timeout/abort signal. If the request never resolves (hung local bridge, network stall), the UI flow in `App.vue` remains stuck after showing the packaging notice and may leave a blank popup tab open.
### Issue Context
Other gateway calls already bound fetch time (e.g. provider models). The handoff path is user-triggered and may involve server-side git inspection, so it should also be time-bounded.
### Fix Focus Areas
- src/api/codexGateway.ts[565-580]
### Suggested fix
- Add an `AbortSignal.timeout(...)` (or AbortController) to the fetch request.
- Convert abort errors into a friendly `Error` message so the existing `App.vue` catch path shows an actionable notice.
- Optionally include a slightly longer timeout than typical metadata calls (e.g. 15–30s) since handoff packaging can be heavier.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+4241
to
+4243
| async function onContinueInChatGptPro(threadId: string): Promise<void> { | ||
| if (threadId !== selectedThreadId.value || !composerCwd.value) return | ||
| const chatWindow = window.open('about:blank', '_blank') |
There was a problem hiding this comment.
3. Pro handoff silent no-op 🐞 Bug ≡ Correctness
The sidebar enables “Continue in ChatGPT Pro…” for any selected thread, but the App.vue handler returns immediately when composerCwd is empty, resulting in a click that does nothing (no notice, no error) for threads without a working directory. This can occur when the selected thread’s cwd is missing/empty.
Agent Prompt
### Issue description
The new menu action can be enabled but perform a silent early return when `composerCwd` is empty, producing a confusing no-op.
### Issue Context
- The menu item is disabled only when the thread is not selected.
- `composerCwd` is derived from `selectedThread.cwd` on the thread route; if that is empty, `onContinueInChatGptPro()` returns before showing any notice.
### Fix Focus Areas
- src/App.vue[4241-4246]
- src/App.vue[1800-1803]
- src/components/sidebar/SidebarThreadTree.vue[627-644]
### Suggested fix
Pick one:
1) In `onContinueInChatGptPro`, replace the silent return for missing `composerCwd` with `showProHandoffNotice('This thread has no working directory to package.', 'error')`.
2) Pass a `hasCwd`/`cwd` boolean down to `SidebarThreadTree` so it can disable the menu item (and set a title) when the selected thread lacks a cwd.
Either approach ensures the user gets immediate feedback instead of nothing happening.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+8076
to
+8083
| const requestedCwd = readNonEmptyString(body?.cwd) | ||
| const cwd = readNonEmptyString(thread?.cwd) || requestedCwd | ||
| if (!cwd || !isAbsolute(cwd)) { | ||
| setJson(res, 400, { error: 'The thread does not have a valid working directory' }) | ||
| return | ||
| } | ||
| const cwdStat = await stat(cwd) | ||
| if (!cwdStat.isDirectory()) { |
There was a problem hiding this comment.
4. Handoff accepts request cwd 🐞 Bug ⛨ Security
The new /codex-api/thread/pro-handoff endpoint falls back to body.cwd when the persisted thread cwd is missing, then uses that value to stat() the directory, read AGENTS.md, and run git commands to build the snapshot included in the returned Markdown. This weakens the endpoint’s trust boundary (client-controlled filesystem target for projectless threads) and also turns common invalid-path cases into a 500 via the outer catch.
Agent Prompt
### Issue description
`/codex-api/thread/pro-handoff` uses `cwd = thread.cwd || body.cwd`. When `thread.cwd` is missing, the caller can choose an arbitrary absolute directory for server-side inspection (stat, git, AGENTS.md reads), and failures (e.g. ENOENT) bubble to the outer catch as HTTP 500.
### Issue Context
This is a local-bridge endpoint, so practical exploitability depends on who can reach the bridge, but it still expands the set of filesystem locations the endpoint will inspect based solely on client input.
### Fix Focus Areas
- src/server/codexAppServerBridge.ts[8062-8093]
- src/server/codexAppServerBridge.ts[4363-4391]
- src/server/proHandoff.ts[191-215]
### Suggested fix
- Prefer removing the `body.cwd` fallback entirely: require `thread.cwd` to exist and be absolute; otherwise return 400 (with an explicit message).
- If a fallback is required for some workflow, validate `body.cwd` against an allowlist (e.g., workspace roots) and/or require it to match the stored thread cwd.
- Wrap `stat(cwd)` in a try/catch and return a controlled 4xx (e.g., 400) for invalid/unreadable directories.
This keeps the handoff snapshot aligned with the actual thread and avoids client-directed filesystem probing.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/server/proHandoff.test.ts (1)
1-91: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the truncation/fallback branches.
Current tests only exercise the happy path and reasoning exclusion. The bounded-output guarantees (
appendBoundedat 360k chars,fencedTextat 12k chars) and therepository: null/ emptycontextFilesfallback messages aren't covered, so a regression in the truncation math wouldn't be caught.✅ Example additional test cases
it('falls back to the not-a-git-repo message when repository is null', () => { const markdown = buildChatGptProHandoff({ threadResult: { thread: {} }, repository: null }) expect(markdown).toContain('The working directory is not inside a Git repository.') }) it('truncates transcripts past the character limit', () => { const hugeText = 'x'.repeat(400_000) const markdown = buildChatGptProHandoff({ threadResult: { thread: { turns: [{ items: [{ type: 'agentMessage', text: hugeText }] }] } }, }) expect(markdown).toContain('Transcript truncated at') })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/server/proHandoff.test.ts` around lines 1 - 91, Add tests in the buildChatGptProHandoff suite covering the repository: null fallback message, empty contextFiles fallback behavior, and transcript truncation with content exceeding appendBounded’s 360k-character limit. Also cover fencedText’s 12k-character truncation path and assert the corresponding truncation markers, while preserving existing happy-path and reasoning-exclusion coverage.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/composables/useDesktopState.ts`:
- Line 1787: Update both fallback retry paths in useDesktopState around
applyFallbackModelSelection() to derive the fallback model’s compatible effort
only after the selection is applied, rather than reusing the previously captured
pending.effort. Pass that recalculated effort to both retry startThreadTurn
calls, and add a regression test covering an ultra turn falling back to a model
with a compatible default effort.
---
Nitpick comments:
In `@src/server/proHandoff.test.ts`:
- Around line 1-91: Add tests in the buildChatGptProHandoff suite covering the
repository: null fallback message, empty contextFiles fallback behavior, and
transcript truncation with content exceeding appendBounded’s 360k-character
limit. Also cover fencedText’s 12k-character truncation path and assert the
corresponding truncation markers, while preserving existing happy-path and
reasoning-exclusion coverage.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 48f5770e-aee7-45c5-a8f2-611c0fe8853f
📒 Files selected for processing (13)
src/App.vuesrc/api/codexGateway.test.tssrc/api/codexGateway.tssrc/components/content/ThreadComposer.vuesrc/components/sidebar/SidebarThreadTree.vuesrc/composables/useDesktopState.test.tssrc/composables/useDesktopState.tssrc/server/codexAppServerBridge.tssrc/server/proHandoff.test.tssrc/server/proHandoff.tssrc/types/codex.tstests/git-worktrees-rollback/thread-menu-copy-chat-action.mdtests/providers-models/per-thread-model-selection.md
| ensureAvailableModelIds(normalizedModelId) | ||
| if (selectedThreadId.value === normalizedThreadId) { | ||
| selectedModelId.value = readModelIdForThread(selectedThreadId.value) | ||
| ensureSelectedReasoningEffortForModel() |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Revalidate effort for fallback retries.
This resets the UI selection for the fallback model, but both fallback retry paths still submit the previously captured pending.effort. A turn started with ultra can therefore retry on the fallback model with unsupported ultra instead of its compatible default. Derive the fallback effort after applyFallbackModelSelection() and use it in both retry startThreadTurn calls; add an ultra fallback regression test.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/composables/useDesktopState.ts` at line 1787, Update both fallback retry
paths in useDesktopState around applyFallbackModelSelection() to derive the
fallback model’s compatible effort only after the selection is applied, rather
than reusing the previously captured pending.effort. Pass that recalculated
effort to both retry startThreadTurn calls, and add a regression test covering
an ultra turn falling back to a model with a compatible default effort.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/App.vue (1)
4251-4272: 🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick winPrevent concurrent handoff generation.
Repeated menu actions before the request settles open multiple windows and start duplicate repository-inspection requests. Add an in-flight guard and clear it in
finally; disable the triggering action while active.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/App.vue` around lines 4251 - 4272, Update onContinueInChatGptPro to use an in-flight guard that returns immediately when a handoff is already being generated, sets the guard before starting the async work, and clears it in a finally block. Bind the triggering menu action’s disabled state to this guard so repeated activations are prevented while the request is pending.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/api/codexGateway.test.ts`:
- Around line 124-127: Remove the duplicate requests declaration and its
corresponding duplicate requests.push call within the test case “uses the
selected opaque credit ID and caller-provided idempotency key,” leaving one
requests collection and one recording operation.
In `@src/api/codexGateway.ts`:
- Around line 1513-1525: The default idempotency key in
consumeRateLimitResetCredit must persist across ambiguous retries instead of
being regenerated on every call. Update the retry/attempt state around
consumeRateLimitResetCredit and createRateLimitResetIdempotencyKey so one key is
retained and reused until an authoritative refresh resolves the attempt, while
preserving explicit idempotencyKey behavior and existing normalization.
In `@src/components/content/RateLimitStatus.vue`:
- Around line 2-6: Update the RateLimitStatus template so resetNotice remains
rendered when resetCredits is null: move the notice rendering outside the
section gated by resetCredits, while preserving the existing snapshots and
refreshed-credits display conditions and the outer aside visibility check.
---
Outside diff comments:
In `@src/App.vue`:
- Around line 4251-4272: Update onContinueInChatGptPro to use an in-flight guard
that returns immediately when a handoff is already being generated, sets the
guard before starting the async work, and clears it in a finally block. Bind the
triggering menu action’s disabled state to this guard so repeated activations
are prevented while the request is pending.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3b48aa2d-fbe7-4411-b407-350bb0df7167
📒 Files selected for processing (12)
src/App.vuesrc/api/codexGateway.test.tssrc/api/codexGateway.tssrc/components/content/RateLimitStatus.vuesrc/composables/useDesktopState.test.tssrc/composables/useDesktopState.tssrc/server/rateLimitDecodeRecovery.test.tssrc/server/rateLimitDecodeRecovery.tssrc/style.csssrc/types/codex.tstests/accounts-feedback-observability/banked-rate-limit-resets.mdtests/accounts-feedback-observability/index.md
Comment on lines
+124
to
+127
| it('uses the selected opaque credit ID and caller-provided idempotency key', async () => { | ||
| const requests: Array<{ method: string; params: Record<string, unknown> }> = [] | ||
| vi.stubGlobal('fetch', vi.fn(async (_input: RequestInfo | URL, init?: RequestInit) => { | ||
| requests.push(JSON.parse(String(init?.body)) as { method: string; params: Record<string, unknown> }) |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Remove the duplicate declarations.
requests is declared twice in the same scope, so TypeScript compilation fails. The duplicate requests.push(...) must also be removed.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/api/codexGateway.test.ts` around lines 124 - 127, Remove the duplicate
requests declaration and its corresponding duplicate requests.push call within
the test case “uses the selected opaque credit ID and caller-provided
idempotency key,” leaving one requests collection and one recording operation.
Comment on lines
+1513
to
+1525
| export async function consumeRateLimitResetCredit( | ||
| creditId?: string, | ||
| idempotencyKey: string = createRateLimitResetIdempotencyKey(), | ||
| ): Promise<UiRateLimitResetOutcome> { | ||
| const normalizedIdempotencyKey = idempotencyKey.trim() | ||
| if (!normalizedIdempotencyKey) throw new Error('A reset idempotency key is required') | ||
| const normalizedCreditId = creditId?.trim() ?? '' | ||
| const params: { idempotencyKey: string; creditId?: string } = { | ||
| idempotencyKey: normalizedIdempotencyKey, | ||
| } | ||
| if (normalizedCreditId) params.creditId = normalizedCreditId | ||
|
|
||
| const result = await callRpc<unknown>('account/rateLimitResetCredit/consume', params) |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Preserve the idempotency key across ambiguous retries.
Each call creates a fresh key, so a retry after a server-side success but lost response is a new consumption. This can redeem another credit when no creditId is supplied. Retain and reuse one attempt key until an authoritative refresh resolves the attempt.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/api/codexGateway.ts` around lines 1513 - 1525, The default idempotency
key in consumeRateLimitResetCredit must persist across ambiguous retries instead
of being regenerated on every call. Update the retry/attempt state around
consumeRateLimitResetCredit and createRateLimitResetIdempotencyKey so one key is
retained and reused until an authoritative refresh resolves the attempt, while
preserving explicit idempotencyKey behavior and existing normalization.
Comment on lines
+2
to
+6
| <aside | ||
| v-if="snapshots.length > 0 || resetCredits !== null || resetNotice !== null" | ||
| class="rate-limit-status" | ||
| aria-live="polite" | ||
| > |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Keep reset outcomes visible when refreshed credits are omitted.
A refresh can set resetCredits to null after the state layer has set resetNotice, but the notice is inside v-if="resetCredits !== null". The outer aside then renders empty, so success or error feedback disappears. Render the notice outside that section.
Proposed fix
- <p
- v-if="resetNotice"
- class="banked-reset-notice"
- :data-type="resetNotice.type"
- :role="resetNotice.type === 'error' ? 'alert' : 'status'"
- >
- {{ resetNotice.text }}
- </p>
</section>
+
+ <p
+ v-if="resetNotice"
+ class="banked-reset-notice"
+ :data-type="resetNotice.type"
+ :role="resetNotice.type === 'error' ? 'alert' : 'status'"
+ >
+ {{ resetNotice.text }}
+ </p>Also applies to: 33-98
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/content/RateLimitStatus.vue` around lines 2 - 6, Update the
RateLimitStatus template so resetNotice remains rendered when resetCredits is
null: move the notice rendering outside the section gated by resetCredits, while
preserving the existing snapshots and refreshed-credits display conditions and
the outer aside visibility check.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
model/listmetadata, preserving server order/defaults and exposing Max/Ultra for models such as GPT-5.6 Sol while retaining a conservative fallback for provider-only modelsAGENTS.mdinstructions, then opens ChatGPTValidation
pnpm exec vitest run src/api/codexGateway.test.ts src/composables/useDesktopState.test.ts src/server/proHandoff.test.ts— 50/50 passed on the clean upstream branchpnpm run build— passednode dist-cli/index.js --help— passedPerformance
Summary by CodeRabbit